Skip to content

docs: say what the count counted, and why these two are not terminal (#1215) - #1245

Merged
jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:docs/1215-name-what-the-count-counted
Sep 24, 2026
Merged

jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:docs/1215-name-what-the-count-counted

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Three comments, no code. From #1215, which I filed as a policy question and which
turned out to be answered already -- the two things left were a stale count and a
missing rationale.

A count that did not say what it counted

arrow_import.sh explains why it does not use pgc_skip, and said:

WHY NOT pgc_skip, WHICH THE OTHER 29 pyarrow SUITES USE.

Recounted, it is 25: the suites whose source, comments stripped, calls
pgc_skip with the pyarrow capability. That reproduces the 25 in #1215's
description, which came from running every suite twice -- two independent routes
to the same number.

29 is not reproducible now under any population I could construct except one
that counts lib.sh, where the function is defined, plus three suites that gate
on fixture/python and name pyarrow only in prose. I cannot show it was
wrong when written
, only that it is not reproducible today, and the comment now
says that instead of carrying a bare number.

The recount is a worked example of the thing it fixes. My first attempt
returned 26:

  grep -c 'pgc_skip' test/arrow_import.sh                  ->  1
  sed 's/#.*//' test/arrow_import.sh | grep -c 'pgc_skip'  ->  0

The 1 is the sentence explaining why the file does not call pgc_skip. Same
defect test/selftest/190 exists for, and the reason the new comment describes
its population in prose rather than pasting a pipeline -- run_all_versions.sh
already records that spelling a shape out in a comment made selftest 400's sweep
flag that very line.

Two deviations that did not say they were deviations

native_parquet_schema.sh and parquet_nested_import.sh decline arm by arm
rather than ending the run, exactly as arrow_import.sh does -- but unlike it,
neither said why. A reader arriving at either could not tell a deliberate choice
from an oversight, which is the state a comment exists to prevent.

Both now carry the measurement that justifies it. Measured on pg18a, each suite
run twice against a pyarrow shimmed to raise ImportError:

  native_parquet_schema    with pyarrow  35 passed +  0 skipped = 35   rc=0
                           without       29 passed +  6 skipped = 35   rc=0
  parquet_nested_import    with pyarrow   7 passed +  0 skipped =  7   rc=0
                           without        5 passed +  2 skipped =  7   rc=0

Identical name sets either way -- 35 and 7 RESULT records in both runs -- and
zero arms passing only without pyarrow, so none of those passes is an
error-expecting arm succeeding because the fixture was missing. Terminal is
right for a suite with nothing else to do, and 29 of 35 and 5 of 7 say neither
of these is that suite.

Verification

  shellcheck -s bash -S error (the three files)        exit=0
  arrow_import.sh            pg18a  rc=0   72 passed + 0 failed = 72
  native_parquet_schema.sh   pg18a  rc=0   35 passed + 0 failed = 35
  parquet_nested_import.sh   pg18a  rc=0    7 passed + 0 failed =  7
  harness_selftest           pg17a  rc=0   1138 checks, 0 FAIL
  harness_selftest           pg18a  rc=0   1138 checks, 0 FAIL
  docs_style.sh                     PASSED

harness_selftest is the arm that matters here, not the three suites: its text
sweeps are what a new comment can break, and a comment carrying shell syntax is
how this tree has been bitten before.

No ledger change: no check added, none renamed, so no row moves. That also means
this does not conflict with #1240 or #1244, which both touch
check_ledger_budget.txt.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP

OffgridwithJD and others added 2 commits September 23, 2026 21:18
…ommandprompt#1215)

`arrow_import.sh` explains why it does not use `pgc_skip` and said "THE OTHER 29
pyarrow SUITES USE" it. Recounted, the number is 25: the suites whose source,
with comments stripped, calls `pgc_skip` with the pyarrow capability. 29 is not
reproducible now under any population except one counting `lib.sh`, where the
function is defined, plus three suites that gate on other capabilities and name
pyarrow only in prose. Whether it was right when written is unknown, and the
comment now says that rather than carrying a bare number.

The recount is a worked example of the defect it fixes. The first attempt
returned 26, because searching `arrow_import.sh` for `pgc_skip` matches the
sentence explaining why it does not call `pgc_skip`. Comments stripped, that
file scores 0.

`native_parquet_schema.sh` and `parquet_nested_import.sh` decline arm by arm
rather than being terminal, as `arrow_import.sh` does, but unlike it said
nothing about why. A reader arriving at either had no way to tell a deliberate
deviation from an oversight. Both now carry the measurement that justifies it:
29 of 35 arms and 5 of 7 need no pyarrow, the name set is identical with and
without it, and both runs still exit 0.

Comments only. No test changed, no check added, no behaviour changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
…ommandprompt#1215)

The previous version of this comment said 29 "is not reproducible now under
any population I could construct" and left whether it was right when written
unknown. Both halves were wrong to leave standing.

Counted in a detached worktree at f738dd4, the commit that wrote the sentence,
excluding arrow_import.sh, lib.sh and the three drivers:

                             at f738dd4   today
    names pyarrow anywhere        34          35
    runs `import pyarrow`         29          30
    calls pgc_skip pyarrow        25          25

So 29 reproduces at that commit, by the `import pyarrow` reading and only that
one. The sentence is about the suites that USE pgc_skip, and 25 did -- then and
now. The number was right for one population and attached to another in the
same breath, which is a more useful thing for the next reader to learn than a
number having gone stale.

Raised by @jdatcmd, who reconstructed the 29. Their message also said it held
"by either pattern"; the wide reading was already 34 at that commit, so only
the narrow one gives 29.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
@OffgridwithJD
OffgridwithJD force-pushed the docs/1215-name-what-the-count-counted branch from e941025 to 6e4e4bc Compare September 24, 2026 03:18
…ompt#1215)

The previous version gave three rows and said 29 reproduces "by the
`import pyarrow` reading and by that reading only -- naming pyarrow anywhere
gave 34 even then". True, and it omitted the predicate that actually separates
them, which is comment stripping.

Measured four ways at f738dd4 and today, same exclusions:

                                     at f738dd4   today
    names pyarrow anywhere               34          35
    names pyarrow in CODE                29          30
    runs `import pyarrow` in code        29          30
    imports it AND calls pgc_skip        25          25

The five in the 34/29 gap name pyarrow only to say they do NOT use it:
native_parquet_stack and parallel_export_parquet hand-craft their fixtures, and
avro_manifest, iceberg_deletes and iceberg_name_mapping gate on other
capabilities. Which is the same house-style trap as searching this file for
`pgc_skip` and matching the sentence that says why not to use it.

Three rows invite the reader to guess which "names pyarrow" means. Two
reviewers guessing differently is how 34 and 29 were taken for a disagreement
when both were right about different populations. @jdatcmd asked for the fourth
row and the predicate on each.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 8bd737bc. Comments and a CHANGELOG entry; no code and no test logic changed, confirmed by reading every added non-comment line.

The four rows re-derived, today's column, on current main

  names pyarrow anywhere          35     PR says 35
  names pyarrow in CODE           30     PR says 30
  runs `import pyarrow` in code   30     PR says 30
  imports it AND calls pgc_skip   25     PR says 25

Same exclusions (arrow_import.sh, lib.sh, the five drivers). I verified the f738dd4d column separately in a worktree at that commit and got 34 / 29 / 29 / 25.

Why the four-row form is the right shape and three was not

34 / 29 / 25 invites the reader to guess which "names pyarrow" means, and that guess is exactly what produced the disagreement: I counted with comments stripped and read 29, the author counted them in and read 34, and neither of us had said so. Two correct numbers, adjacent questions, no conflict to resolve once the predicate was named.

Naming the predicate on each row makes the comment a demonstration of its own subject rather than a correction of a predecessor. The count was never wrong; the noun was.

The conclusion is the durable half

The sentence said "THE OTHER 29 pyarrow SUITES USE pgc_skip". At the commit that wrote it, 29 suites other than that one ran import pyarrow and 25 of them used pgc_skip. The number was right for one population and attached to another in the same breath.

25 has not moved since. The count that moved is the one the sentence did not mean: 29 to 30, because capability_sweep.sh joined the pyarrow population when #1235 landed — which is the self-exclusion in that sweep doing its job in the arithmetic a day later, visible only from outside.

Recording "this counted X at time T" rather than implying a predecessor was careless is right, and it is what lets the next reader learn the transferable thing instead of a fact about one comment.

@jdatcmd
jdatcmd merged commit a0a6c30 into commandprompt:main Sep 24, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants